Skip to content

Conversation

@diegorusso
Copy link
Contributor

@diegorusso diegorusso commented Dec 5, 2025

@diegorusso diegorusso changed the title GH-142305: JIT: Deduplicating GOT symbols in the trace gh-142305: JIT: Deduplicating GOT symbols in the trace Dec 8, 2025
@diegorusso diegorusso changed the title gh-142305: JIT: Deduplicating GOT symbols in the trace GH-142305: JIT: Deduplicating GOT symbols in the trace Dec 8, 2025
Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question about the design choice!

Meta comment: man, I feel like GitHub has done this to me twice now where it's posted stale comments and/or eaten comments I've written during review...

@bedevere-app
Copy link

bedevere-app bot commented Dec 8, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.


// Get the symbol slot memory location for a given symbol ordinal.
static unsigned char *
get_symbol_slot(int ordinal, symbol_state *state, int size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, merging GOT and trampoline symbols into the same structure makes things a bit more challenging to understand, since they serve different purposes. Would it be worth keeping them as separate structures with their own ordinals? I know it means some duplication, but it might be easier to reason about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the symbol map is just to track the symbols available, independently how they are reached (either by GOT or trampolines). In fact the symbol map won't change at all, it's just an enumeration of the symbols across different stencils.

The way that differentiates how they are reached is the relocation tag associated to it. For example:

  • R_AARCH64_CALL26 will use trampolines
  • R_AARCH64_ADR_GOT_PAGE/R_AARCH64_LD64_GOT_LO12_NC will use the GOT

but the ordinal of the symbol is consistent because the collection of the symbol is unified.
Indeed in the trace, trampolines and got entries are allocated in separate areas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants